Skip to content

internal: Utilize cargo check --compile-time-deps #20047

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 21, 2025
Merged

internal: Utilize cargo check --compile-time-deps #20047

merged 1 commit into from
Jun 21, 2025

Conversation

ShoyuVanilla
Copy link
Member

@ShoyuVanilla ShoyuVanilla commented Jun 19, 2025 •
edited
Loading

rust-lang/cargo#15674 has been merged and I guess it would be available in toolchain 1.90.0

I've tested this on rust-analyzer itself with cargo binary built from current master with the following my rust-analyzer branch
https://github.com/ShoyuVanilla/rust-analyzer/tree/comp-time-deps-test
and these are the results (I had run cargo clean before each run and modified the code a bit to use --compile-time-deps with injected cargo binary path iff env var $CARGO_BIN_FOR_TEST is set)

With --compile-time-deps

image

Without --compile-time-deps

image

Comparison

analysis-stat results Time target dir size
--compile-time-deps Identical 2m 25s 347 M
default Identical 2m 55s 1.4 G

System Information

image

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 19, 2025
@lnicola
Copy link
Member

lnicola commented Jun 19, 2025

😍

@edwin0cheng
Copy link
Contributor

Do we want to disable RUSTC_WRAPPER too ?

@Veykril
Copy link
Member

Veykril commented Jun 21, 2025

We should not set the wrapper if we use the flag

@ShoyuVanilla
Copy link
Member Author

Do we want to disable RUSTC_WRAPPER too ?

Oh, I missed it. Thanks!

@Veykril
Copy link
Member

Veykril commented Jun 21, 2025

This is amazing, can't wait to get rid of the wrapper in the future

@Veykril Veykril added this pull request to the merge queue Jun 21, 2025
Merged via the queue into rust-lang:master with commit 56de211 Jun 21, 2025
14 checks passed
@ShoyuVanilla ShoyuVanilla deleted the comp-time-deps branch June 21, 2025 10:05
@bjorn3
Copy link
Member

bjorn3 commented Jul 11, 2025

Does this work in the rust-lang/rust workspace too?

@ShoyuVanilla
Copy link
Member Author

Does this work in the rust-lang/rust workspace too?

Haven't tested in it yet but I guess so?

@bjorn3
Copy link
Member

bjorn3 commented Jul 11, 2025

We set rust-analyzer.cargo.buildScripts.overrideCommand to ./x.py check --json-output. ./x.py check --json-output -Zunstable-options --compile-time-deps is not a valid command, which makes it seem to me like cargo_comp_time_deps_available is false (maybe toolchain is even None). We should probably add support for --compile-time-deps to x.py and then we could set it in the rust-analyzer config, but at the same time rust-analyzer should avoid setting RUSTC_WRAPPER.

@ShoyuVanilla
Copy link
Member Author

Oh, I forgot about those x scripts in rust. Currently rust-analyzer is not appending --compile-time-deps when overrideCommands is set. #20121 (And setting that also ignores RUSTC_WRAPPER, so I guess currently in rust-lang/rust, neither --compile-time-deps nor RUSTC_WRAPPER is executed and thus full cargo check is happening 🙃 )

So, adding supports for --compile-time-deps to ./x.py sounds just enough to me.

@bjorn3
Copy link
Member

bjorn3 commented Jul 11, 2025

Opened rust-lang/rust#143785. Saves a lot of time!

@ShoyuVanilla
Copy link
Member Author

That's way better than my naive expectations 🎉

@ogapo ogapo mentioned this pull request Jul 17, 2025
Open
@Veykril Veykril mentioned this pull request Jul 21, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Veykril Veykril Veykril approved these changes

Assignees
No one assigned
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ShoyuVanilla @lnicola @edwin0cheng @Veykril @bjorn3 @rustbot